-
Notifications
You must be signed in to change notification settings - Fork 52
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: removed wallet_getCapabilities #104
Conversation
There are 2 ways we can set the address in the capabilities var:
|
crates/e2e-tests/src/tests.rs
Outdated
let capabilities = { | ||
let mut innermost_tree = BTreeMap::new(); | ||
innermost_tree.insert( | ||
"addresses".to_string(), | ||
vec![Address::from_str(&std::env::var("DELEGATION_ADDRESS").unwrap()).unwrap()], | ||
); | ||
|
||
let mut inner_tree = BTreeMap::new(); | ||
inner_tree.insert("delegation".to_string(), innermost_tree); | ||
|
||
let mut capabilities = BTreeMap::new(); | ||
capabilities.insert(chain_id, inner_tree); | ||
|
||
capabilities | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should remove the capabilities structs entirely, we don't need them anymore, and instead use a const address here or an env var
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to clarify, you mean to remove WalletCapabilities
right?
#[derive(Debug, Clone, Eq, PartialEq, Deserialize, Serialize)]
pub struct WalletCapabilities(pub HashMap<U64, Capabilities>);
I see its not really being used anywhere else in the code
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes
@varun-doshi looks good overall, some lints are failing PTAL |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sorry about this, can you resolve the merge conflicts as well? I can't push to your PR since you opened it from your main branch
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
still some failing lints, maybe due to merge? sorry about this and thank you for your patience 🥲 if you open up your PR from another branch than main next time I can push to your pr to handle the nits
Understood |
Opened new PR with correct branch #110 |
Closes #99 Removed `get_capabilities` from the `WalletCapabilities` trait In `e2e_tests` the capabilities is initialised manually with the contract address being pulled in from `env` variables. Related PR: #104 (review) --------- Co-authored-by: Oliver Nordbjerg <[email protected]>
Closes #99
Removed
get_capabilities
from theWalletCapabilities
traitIn
e2e_tests
the capabilities is initialised manually with the contract address being pulled in fromenv
variables.